Skip to content

Conversation

@ogadra
Copy link
Contributor

@ogadra ogadra commented Nov 15, 2024

Related #21

Support for asynchronous getLoadContext.
The getLoadContext function is already awaited in src/middleware.ts at line 21.
Therefore, we only need to support the type that returns Promise<AppLoadContext>.

※ 英語に自信がないので、日本語でも説明します。
非同期の getLoadContext のサポート。
getLoadContext 関数はすでに src/middleware.tsの21行目でawaitされています。
そのため、GetLoadContext 型が Promise<AppLoadContext> を返せるようにすればよいです。

@ogadra ogadra changed the title Support for asynchronous getLoadContext feat: Support for asynchronous getLoadContext Nov 15, 2024
@yusukebe
Copy link
Owner

Hi @ogadra Thank you for the PR!

@predaytor Will this solve your problem? Can you review this?

@predaytor
Copy link

predaytor commented Nov 16, 2024

@yusukebe yeah! except we also need to check the promise in dev script at src/dev.ts:

app.all('*', async (c) => {
  // ...

  // const remixContext = getLoadContext(args)
  // return handler(c.req.raw, remixContext)

  const remixContext = getLoadContext(args)

  return handler(c.req.raw, remixContext instanceof Promise ? await remixContext : remixContext)
})

@yusukebe
Copy link
Owner

@ogadra @predaytor

I've updated to check if Promise or not and added the type defaultGetLoadContext: 37cda75

Can you review this?

@ogadra
Copy link
Contributor Author

ogadra commented Nov 17, 2024

@yusukebe Thanks for support! Looks good to me!

@yusukebe
Copy link
Owner

Okay! Merging.

@yusukebe yusukebe merged commit b0e39e2 into yusukebe:main Nov 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants